Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: better error when Sharp can't be resolved (ex: pnpm) #8128

Merged
merged 3 commits into from
Aug 18, 2023

Conversation

Princesseuh
Copy link
Member

Changes

Unfortunately, due to bundling, packaging, JavaScript, etc nonsense, on pnpm Sharp fails to be found. There's no proper way to fix this that wouldn't be: risky and/or unreliable. Trust me, I tried putting sharp in every single Vite config I found.

So, I did the next best thing: I upgraded the error message with every bit of info I could. I also added the passthrough / noop image service to the config so it's easier to use for SSR users who don't want to install Sharp into their projects. In the future, a more optimal solution for SSR would be a way to disable the endpoint, but that also requires either rethinking how we do images in Markdown, or at least a way to disable image processing in Markdown, it's a bit complex.

Fix #7966

Testing

Tested manually

Docs

/cc @withastro/maintainers-docs for feedback on the error message and description

@Princesseuh Princesseuh requested a review from a team as a code owner August 18, 2023 11:08
@changeset-bot
Copy link

changeset-bot bot commented Aug 18, 2023

🦋 Changeset detected

Latest commit: bcc567a

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Aug 18, 2023
Copy link
Member

@ElianCodes ElianCodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this great error, @Princesseuh ! I added some suggestions for details in case you wanted to make these even more super-explicit, as well as some links I know will be live in the new docs. (Some are now.) Your call!

My only concern was that (as you know from my trying to write these new docs!) "image processing" can risk sounding vague if people don't really get that "processed" doesn't include "simply display on your site". So this message makes me wonder in general maybe whether referring to "using astro:assets or not?" is an easier way for readers to understand that "processing" is more than just "agreeing to display your image on your page." 😄

packages/astro/src/core/errors/errors-data.ts Outdated Show resolved Hide resolved
packages/astro/src/core/errors/errors-data.ts Outdated Show resolved Hide resolved
packages/astro/src/core/errors/errors-data.ts Outdated Show resolved Hide resolved
packages/astro/src/core/errors/errors-data.ts Outdated Show resolved Hide resolved
Co-authored-by: Sarah Rainsberger <[email protected]>
@Princesseuh Princesseuh merged commit c2c71d9 into next Aug 18, 2023
@Princesseuh Princesseuh deleted the feat/better-sharp-error branch August 18, 2023 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants